Skip to content

fix(test): replace Collection|iterable with Collection and add appropriate PHPDoc tags #7303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vinceAmstoutz
Copy link
Member

@vinceAmstoutz vinceAmstoutz commented Jul 19, 2025

Q A
Branch? main
Tickets -
License MIT

Fixes several phpstan errors ignored in the config

@vinceAmstoutz vinceAmstoutz force-pushed the fix/remove-call-method-add-on-iterable-phpstan branch 2 times, most recently from 0e63fe2 to b890688 Compare July 21, 2025 06:50
@vinceAmstoutz vinceAmstoutz force-pushed the fix/remove-call-method-add-on-iterable-phpstan branch 3 times, most recently from 0062175 to d4eb6d4 Compare July 25, 2025 14:33
@soyuka
Copy link
Member

soyuka commented Aug 18, 2025

ci is all red probably due to this change

@vinceAmstoutz vinceAmstoutz force-pushed the fix/remove-call-method-add-on-iterable-phpstan branch 2 times, most recently from 091574a to 7fe03c9 Compare August 22, 2025 14:29
…propriate PHPDoc tags

Fixes several phpstan errors ignored in the config
@vinceAmstoutz vinceAmstoutz force-pushed the fix/remove-call-method-add-on-iterable-phpstan branch from 7fe03c9 to d35038a Compare August 22, 2025 14:38
@vinceAmstoutz
Copy link
Member Author

vinceAmstoutz commented Aug 22, 2025

ci is all red probably due to this change

@VincentLanglet @soyuka At some points both Collection and iterable types are used from different places for the same property, so I found a solution to prevent typing error from PHPStan, here an example.
WDYT?

Otherwise I have this strange failure in Behat checks but I only edited fixtures files (entities & documents), any idea?
image

@VincentLanglet
Copy link
Contributor

@VincentLanglet @soyuka At some points both Collection and iterable types are used from different places for the same property, so I found a solution to prevent typing error from PHPStan, here an example.

WDYT?

I feel like it's really hacky and kinda a code smell to have an "addFoo" method which does nothing when the property is an iterable.

Ok it's "only for tests files" but it could still lead to some weird error in the futur, and adding such hack just to fix a phpstan baseline error might not be worth it.

Otherwise I have this strange failure in Behat checks but I only edited fixtures files (entities & documents), any idea?

image

It could be more insteresting to use the first fix you use (Collection everywhere) and understand the behat failure. I can only take a look starting next week since i'm currently one vacation without a computer.

@vinceAmstoutz
Copy link
Member Author

vinceAmstoutz commented Aug 23, 2025

@VincentLanglet @soyuka At some points both Collection and iterable types are used from different places for the same property, so I found a solution to prevent typing error from PHPStan, here an example.

WDYT?

I feel like it's really hacky and kinda a code smell to have an "addFoo" method which does nothing when the property is an iterable.

Ok it's "only for tests files" but it could still lead to some weird error in the futur, and adding such hack just to fix a phpstan baseline error might not be worth it.

Otherwise I have this strange failure in Behat checks but I only edited fixtures files (entities & documents), any idea?

image

It could be more insteresting to use the first fix you use (Collection everywhere) and understand the behat failure. I can only take a look starting next week since i'm currently one vacation without a computer.

Ok thanks @VincentLanglet , I will revert to Collection everywhere, but probably we'll need to adapt some parts which uses iterables at some parts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants